Fix type mismatch in StreamCancelFn assignment#2715
Fix type mismatch in StreamCancelFn assignment#2715stevenvo wants to merge 1 commit intowavetermdev:mainfrom
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughTwo files were modified to update the signature of the Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-01-22T01:28:41.417ZApplied to files:
📚 Learning: 2025-01-22T22:27:25.739ZApplied to files:
🪛 golangci-lint (2.5.0)pkg/wshrpc/wshclient/wshclientutil.go[error] 66-66: : # github.com/wavetermdev/waveterm/pkg/wshrpc/wshclient (typecheck) 🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| opts.StreamCancelFn = func() { | ||
| reqHandler.SendCancel(context.Background()) |
There was a problem hiding this comment.
Stream cancel callback still mismatches web handler signature
The new zero-arg StreamCancelFn here still does not match how it is consumed: handleRemoteStreamFileFromCh in pkg/web/web.go expects a func(context.Context) error and passes that type through rpcOpts.StreamCancelFn, so after this change the web package will still fail to compile with a type error (func() vs func(context.Context) error). This patch therefore doesn’t resolve the build break that motivated it; building the web server will still halt at that call site.
Useful? React with 👍 / 👎.
StreamCancelFn was incorrectly defined as func() but is actually used as func(context.Context) error throughout the codebase. This caused build failures in multiple places: - wshclientutil.go:66: cannot use func(ctx context.Context) error as func() value - web.go:255: cannot use func() as func(context.Context) error value Fixed by correcting the type definition in wshrpctypes.go to match actual usage. Fixes build errors introduced in wavetermdev#2709
d41b118 to
d8f3b32
Compare
|
Closing to create new PR with accurate description. Initial analysis was incorrect - the fix is simpler than described. |
Summary
Fixes build error introduced in #2709 where
StreamCancelFnassignment has incorrect function signature.Problem
Build fails with:
Root Cause
StreamCancelFnis defined asfunc()inwshrpctypes.go:376func(ctx context.Context) errorinwshclientutil.go:66Fix
Match the expected signature by:
context.Background()for the SendCancel callTesting
Build now succeeds without type errors.
Related